revamp dotEnv parser#3347
Merged
Jarred-Sumner merged 1 commit intooven-sh:mainfrom Jun 23, 2023
Merged
Conversation
Collaborator
|
Nice. This mostly looks good. Can we look at a perf comparison? Maybe just paste a file with the same text copy pasted a thousand times and then paste timings of .env before & after Performance matters for this because it’s used everywhere in Bun. So it can’t become a bottleneck. It impacts buns install, bun run, the runtime, basically everything |
alexlamsl
force-pushed
the
dot-env
branch
9 times, most recently
from
9f05174 to
5c014f0
Compare
Collaborator
There was a problem hiding this comment.
there's technically a pointer artihmetic thing we can do here
but pointer arithmetic is an easy way to segfault
Contributor
Author
There was a problem hiding this comment.
They do that in indexOfNewlineOrNonASCIIOrANSI() etc. − however they also do slice.len - remaining.len a few lines above (first cannot be a pointer after all), so I opt for consistency/readability here.
alexlamsl
force-pushed
the
dot-env
branch
2 times, most recently
from
468eef0 to
bdccd7e
Compare
- fixes `strings.indexOfAny()` - fixes OOB array access fixes oven-sh#411 fixes oven-sh#2823 fixes oven-sh#3042
Collaborator
|
Thank you for this |
Jarred-Sumner
pushed a commit
that referenced
this pull request
Jun 24, 2023
Jarred-Sumner
added a commit
that referenced
this pull request
Jun 24, 2023
* wip changes for CommonJS * this rewrite is almost complete * even more code * wip * Remove usages of `import.meta.require` from builtins * Remove usages of require * Regenerate * ✂️ builtin rewrite commonjs in printer * Use lazy custom getters for import.meta * fixups * Remove depd * ugh * still crashing * fixup undici * comment out import.meta.require.resolve temporarily not a real solution but it stops the crashes * Redo import.meta.primordials * Builtins now have a `builtin://` protocol in source origin * Seems to work? * Finsih getting rid of primordials * switcharoo * No more function * just one more bug * Update launch.json * Implement `require.main` * ✂️ * Bump WebKit * Fixup import cycles * Fixup improt cycles * export more things * Implement `createCommonJSModule` builtin * More exports * regenerate * i broke some stuff * some of these tests work now * We lost the encoding * Sort of fix zlib * Sort of fix util * Update events.js * bump * bump * bump * Fix missing export in fs * fix some bugs with builtin esm modules (stream, worker_threads, events). its not perfect yet. * fix some other internal module bugs * oops * fix some extra require default stuff * uncomment this file but it crsahes on my machine * tidy code here * fixup tls exports * make simdutf happier * Add hasPrefix binding * Add test for `require.main` * Fix CommonJS evaluation order race condition * Make node:http load faster * Add missing exports to tls.js * Use the getter * Regenerate builtins * Fix assertion failure in Bun.write() * revamp dotEnv parser (#3347) - fixes `strings.indexOfAny()` - fixes OOB array access fixes #411 fixes #2823 fixes #3042 * fix tests for `expect()` (#3384) - extend test job time-out for `darwin-aarch64` * `expect().resolves` and `expect().rejects` (#3318) * Move expect and snapshots to their own files * expect().resolves and expect().rejects * Fix promise being added to unhandled rejection list * Handle timeouts in expect(<promise>) * wip merge * Fix merge issue --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com> Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> * fixup min/memcopy (#3388) * Fix crash in builtins * Don't attempt to evaluate modules with no source code * Update WebCoreJSBuiltins.cpp * Update WebCoreJSBuiltins.cpp * Update WebCoreJSBuiltins.cpp * Fix crash * cleanup * Fix test cc @paperdave * Fixup Undici * Fix issue in node:http * Create util-deprecate.mjs * Fix several bugs * Use the identifier * Support error.code in `util.deprecate` * make the CJs loader slightly more resilient * Update WebCoreJSBuiltins.cpp * Fix macros --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Co-authored-by: dave caruso <me@paperdave.net> Co-authored-by: Alex Lam S.L <alexlamsl@gmail.com> Co-authored-by: Ashcon Partovi <ashcon@partovi.net> Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
strings.indexOfAny()fixes #411
fixes #2823
fixes #3042
fixes #3380